Fix ambiguous trait dispatch when ADTs share constructor names - #68
Merged
Conversation
Trait dispatch keyed on runtime tag string, not static type. With two ADTs sharing a constructor name (e.g. Shape and Blob both have Circle), calling a trait method polymorphically could run the wrong impl. Fix: for monomorphic call sites where the static type is known, emit a direct call to the correct impl (no runtime dispatch needed). For polymorphic sites that fall back to the dispatcher, add a compile-time check (check_ambiguous_dynamic_dispatch) that rejects the program if that method's impls have colliding constructor names, pointing at the call. This is a compile error rather than silently running the wrong code. The guard is narrow: only fires when a polymorphic site meets impls with clashing tags. Polymorphic dispatch over distinct constructors still works. No false positives across the suite including prelude min/max/compare. 5 new tests in regress_trait_dispatch.rs. Full compiler suite green. Note: operator paths (==/</+) still dispatch on tag and have the same latent bug — flagged in AGENTS.md for a separate change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Trait dispatch keyed on runtime tag string, not static type. With two ADTs sharing a constructor name (e.g.
ShapeandBlobboth haveCircle), calling a trait method polymorphically could run the wrong impl — silent wrong output.Fix
check_ambiguous_dynamic_dispatchthat rejects the program if that method's impls have colliding constructor names — a compile error rather than silently running the wrong codeThe guard is narrow: only fires when a polymorphic site meets impls with clashing tags. Polymorphic dispatch over distinct constructors still works. No false positives across the suite including prelude
min/max/compare.Note
Operator paths (
==/</+) still dispatch on tag and have the same latent bug — flagged in AGENTS.md for a separate change.Tests
5 new tests in
regress_trait_dispatch.rs: the reported bug, multi-arg method, bare-value (map area) path, ambiguous-polymorphic rejection, and a control that the guard doesn't overfire.Verification